Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve handling of quota limit reached errors #610

Conversation

Walz
Copy link
Collaborator

@Walz Walz commented Jul 5, 2023

closes #309

Problem

The output of ggshield when the quota limit is reached can be confusing:

$ ggshield secret scan path setup.py
Scanning Path... ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━   0% 0 files scanned out of 1 -:--:--
Scanning failed. Results may be incomplete.
The following chunk is affected:
/Users/sguillaume/gg-src/ggshield/setup.py
403:Quota limit reached.
Scanning Path... ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100% 1 files scanned out of 1 0:00:00

No secrets have been found

Even more with the JSON output:

$ ggshield secret scan path setup.py --json
Scanning Path... ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━   0% 0 files scanned out of 1 -:--:--
Scanning failed. Results may be incomplete.
The following chunk is affected:
/Users/sguillaume/gg-src/ggshield/setup.py
403:Quota limit reached.
Scanning Path... ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100% 1 files scanned out of 1 0:00:00
{"id": "/Users/sguillaume/gg-src/ggshield/setup.py", "type": "path_scan", "total_incidents": 0, "total_occurrences": 0}

The error code is 0, and ggshield say that no secret have been found, when no file have been scanned.

Solution

I've added an exception QuotaLimitReachedError which is raised by SecretScanner._collect_results when ggshield receives a 403 with the message Quota limit reached.. This exception is then passed all the way to up and scan_commit_range has been updated to stop as soon as an exception occurs.

The new output is:

▶ ggshield secret scan path setup.py       
Scanning Path... ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100% 1 files scanned out of 1 0:00:00

Error: Could not perform the requested action: no more API calls available.

The output handler is not called and the exit code is 128.

It also stop a large scan if the error happens in the middle of the scan:
demo


The PR also include a small commit to fix the a version specifier in the Pipfile. My version of pipenv didn't like the missing ==.

@Walz Walz requested a review from agateau-gg July 5, 2023 13:27
Copy link
Collaborator

@agateau-gg agateau-gg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor remarks, looks good. I like the new AbstractGGAPIHandler 👍🏻.

ggshield/secret/repo.py Outdated Show resolved Hide resolved
ggshield/secret/secret_scanner.py Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Jul 5, 2023

Codecov Report

Merging #610 (4f0ccbc) into main (7e5c941) will increase coverage by 0.01%.
The diff coverage is 88.88%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##             main     #610      +/-   ##
==========================================
+ Coverage   93.84%   93.85%   +0.01%     
==========================================
  Files         104      104              
  Lines        4844     4853       +9     
==========================================
+ Hits         4546     4555       +9     
  Misses        298      298              
Flag Coverage Δ
unittests 93.85% <88.88%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
ggshield/secret/repo.py 95.31% <66.66%> (-1.47%) ⬇️
ggshield/core/errors.py 89.85% <100.00%> (+0.46%) ⬆️
ggshield/secret/secret_scanner.py 91.80% <100.00%> (+0.13%) ⬆️

... and 4 files with indirect coverage changes

Walz added 2 commits July 5, 2023 18:05
When quota limit is reached ggshield:
 - prints this error: "Could not perform the requested action: no more API calls available."
 - exits with an error code of 128
 - does not print anything through the output handlers (no text report, no JSON report)
@Walz Walz force-pushed the samuel/scrt-3734-ggshield-improve-handling-of-quota-limit-reached-errors branch from 4f0ccbc to 10fef6f Compare July 5, 2023 16:05
@Walz Walz requested a review from agateau-gg July 5, 2023 16:14
Copy link
Collaborator

@agateau-gg agateau-gg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@agateau-gg agateau-gg merged commit 6cafeb4 into main Jul 6, 2023
24 checks passed
@agateau-gg agateau-gg deleted the samuel/scrt-3734-ggshield-improve-handling-of-quota-limit-reached-errors branch July 6, 2023 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Scans Limit per month
3 participants